-
-
Notifications
You must be signed in to change notification settings - Fork 343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to use uv pip install
#2957
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2957 +/- ##
==========================================
- Coverage 99.63% 99.61% -0.02%
==========================================
Files 121 121
Lines 17841 17841
Branches 3206 3206
==========================================
- Hits 17776 17773 -3
- Misses 46 48 +2
- Partials 19 20 +1
|
This seems nontrivial, can we break this into 2 prs?
Personally I see (1) as much more valuable-- (2) doesn't change much. Personally I'm more excited by uv's ability to get the lowest compatible version so we can add a new CI run than by it replacing |
uv
. Closes #2956uv pip install
Also fix shellcheck reported issues
Windows CI is failing because uv is trying to change it's own executable while running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of your changes remind me that we should probably run shellcheck over our scripts...
ci.sh
Outdated
|
||
python -m build | ||
python -m pip install dist/*.whl | ||
wheel_package=$(ls dist/*.whl) | ||
python -m uv pip install "trio @ $wheel_package" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably lead the path with ./ to get this to work like before: astral-sh/uv#6052
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason not to leave it at my current solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal preference TBH. Maybe you disagree but I prefer moving logic out of shell scripts and into program arguments.
Feel free not to do it though.
Problem is, it's written in haskell and the pre-commit integration for it uses docker, which is a mess |
Yeah I think it's fine not to check it in CI. It's not like our shell scripts are that important or change often; it's just a nicety. |
This pull request follows up on #2956 and is attempting to switch the continuous integration system to use astral.sh' new tool uv.